-
Notifications
You must be signed in to change notification settings - Fork 8
body_read_stream implementation (replacement for PR 57) #78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
body_read_stream implementation (replacement for PR 57) #78
Conversation
|
An automated preview of the documentation is available at https://78.beast2.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2025-11-13 14:50:35 UTC |
13961ee to
b0b43e2
Compare
test/unit/body_read_stream.cpp
Outdated
| auto lambda2 = [&](system::error_code ec, std::size_t n) | ||
| { | ||
| BOOST_TEST_EQ(n, body_length_); | ||
| BOOST_TEST(!ec.failed()); | ||
| BOOST_TEST(pr.got_header()); | ||
| BOOST_TEST(pr.is_complete()); | ||
| invoked2++; | ||
| }; | ||
|
|
||
| int invoked1 = 0; | ||
|
|
||
| auto lambda1 = [&](system::error_code ec, std::size_t n) | ||
| { | ||
| BOOST_TEST_EQ(n, 0); | ||
| BOOST_TEST_EQ(ec, asio::error::operation_aborted); | ||
| BOOST_TEST(pr.got_header() == (i >= header_length_)); | ||
| BOOST_TEST(!pr.is_complete()); | ||
| invoked1++; | ||
| // Append the remainder of the message and try again. | ||
| std::string remainder = msg_.substr(i); | ||
| ts.append(remainder); | ||
| brs.async_read_some( | ||
| buf.prepare(1024), | ||
| lambda2); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks unnecessarily complicated. Do these handlers need to be invoked recursively? Couldn't the same coverage be achieved with a simpler case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I split it into two to make it more readable, with an intervening test::run(ioc)
b0b43e2 to
9f34f0b
Compare
9f34f0b to
d7fdade
Compare
d7fdade to
3a6aa4a
Compare
| { | ||
| namespace beast2 | ||
| { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formatting:
namespace boost {
namespace beast2 {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to force this with a custom .clang-format:
BraceWrapping:
AfterCaseLabel: true
AfterClass: true
AfterControlStatement: Always
AfterEnum: true
AfterExternBlock: true
AfterFunction: true
AfterNamespace: false
AfterObjCDeclaration: true
AfterStruct: true
AfterUnion: true
BeforeCatch: true
BeforeElse: true
BeforeLambdaBody: true
BeforeWhile: true
IndentBraces: false
SplitEmptyFunction: true
SplitEmptyRecord: true
SplitEmptyNamespace: true
BreakBeforeBraces: Custom
| std::string header_ = "HTTP/1.1 200 OK\r\n" | ||
| "Content-Length: 12\r\n" | ||
| "\r\n"; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formatting:
std::string header_ =
"HTTP/1.1 200 OK\r\n"
"Content-Length: 12\r\n"
"\r\n";
We use a single indent for each line break, I couldn't find a way to config this in the clang-format without breaking other stuff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is sufficiently unusual to be outside the scope of clang-format :-)
Done manually.
| // Official repository: https://github.com/cppalliance/beast2 | ||
| // | ||
|
|
||
| #ifndef BOOST_HTTP_IO_IMPL_BODY_READ_STREAM_HPP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BOOST_BEAST2...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ouch! fixing.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #78 +/- ##
===========================================
+ Coverage 37.13% 38.65% +1.51%
===========================================
Files 41 42 +1
Lines 1535 1573 +38
===========================================
+ Hits 570 608 +38
Misses 965 965
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| retained by the caller, which must guarantee that it remains | ||
| valid until the handler is called. | ||
| */ | ||
| explicit body_read_stream(AsyncReadStream& us, http_proto::parser& pr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please replace uses of us with s , including in the documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also make the formatting of the documentation match the docs in read.hpp.
|
GCOVR code coverage report https://78.beast2.prtest3.cppalliance.org/gcovr/index.html Build time: 2025-11-13 15:56:03 UTC |
This is a replacement PR for #57 as the source branch had to be recreated.